Git / GitHub を使用したチーム開発時のガイドラインを制定しました
開発時にはみなさん Git や GitHub を使うと思いますが、使い方についてチームメンバー間で微妙に認識の違いがあると進捗を妨げてしまいます。それを防ぐためにガイドラインを定めてみました。 ちなみにこれは CX 事業本部の Tech Lead のお仕事紹介第 1 弾のポストです。
この記事の英語版も書きました。
前提
CX 事業本部ではクライアントからの開発案件や自社サービスの開発をしていますが、その際に有用な（と考えている）ガイドラインです。
様々な事情でチームメンバーが変更になる可能性があり、新規メンバーの立ち上がりを支援する意味合いも込めています。そのため、開発効率をなるべく落とさずに効果的なスキルトランスファーが実施できることを主眼としています。
ガイドライン
定めたガイドラインの全文を貼ります。
3 つのセクションに分かれています。
commit 時のガイドライン
avoid committing tangled changes
tangled changes というのは複数の意図や目的を持つ変更のことです。ソフトウェア工学における言葉なのですが、 2 単語で言いたいことを言えるので使っています。厳密な定義は次のような論文を読むと良いでしょう。
これらの論文はソフトウェア工学の研究者が書いているので、彼らにとってのデメリット、つまり分析がしづらいという論点も含まれています。現場の開発者にとってのデメリットは単純に、 commit の意図が見えづらくなることと、 revert できなくなることです。
というわけで、僕の関わるプロジェクトでは tangled commit をなくしてくれ、というお願いをチームメンバーにしています。
commit changes to build and pass unit-tests
ユニットテストが通る単位で commit してくれ、というお願いです。
これは暗に commit 時点でテストを書いてくれ、ということも含んでいます。が、 TDD のように先にテストを書くべきという原理主義的なものではなく、設計のための治具としてのテストを指しています。
make a lot of small commits rather than a huge commit
これはコードレビュー経験のある方ならわかるかと思いますが、大きな Pull Request はとにかくレビューが進まないです。前提知識の学習から前提となる変更の確認、本筋の変更の特定など本質的な作業の他に、書き方の指摘や変更内容の方向性の検討などその他の作業を含めるとかなりの重労働となります。
こういった、コードレビューの重労働化を防ぐために、大きな変更をひとつ commit するのではなく、小さな変更を積み重ねて目的を果たしてもらいたいという思いのガイドラインです。
大体、次のような作業をしてもらえれば問題なくレビューが進むと思っています。
- （テストがなければ）テスト拡充
- リファクタリング
- 前提の変更
- 本筋の変更
- リファクタリングによる整理
0 番と 1 番は別の Pull Request でもよいです。
Pull Request 作成時のガイドライン
これらは Pull Request テンプレートで入力を補助すると実践しやすいです。
write related issue number
Pull Request の概要欄にどの issue が関連するかを書きましょう。というものです。
Pull Request の前提や目的を issue が代弁してくれる場合も多いため、レビュワーの理解の手がかりとなります。
また、特定のキーワードをつけると Pull Request がマージされた場合に指定した issue を閉じる機能もありますので活用しましょう。
show screenshots before / after the changes when you make any changes about UI
UI の変更などはコードからでは適切な変更なのか判断がつかない場合があります。
レビュワーの負担を減らすために、変更前と変更後のスクリーンショットをあわせて貼るとよいでしょう。動きのある部分の変更の場合、動画を添えるとよいでしょう。
merge guideline
merging PRs needs 1 or more approvements
GitHub の機能として、ブランチの保護機能があり、その中に マージにあたってレビュワーによる approve が必要という設定項目があります。
これを有効化し、レビューを必須としましょう。レビューはチームにおけるコードに対する理解を深め、属人性を排除するとともに、またとない学習機会でもあります。レビューの実施をポジティブに捉え、効率的に活用しましょう。
merge PRs with creating a merge commit
GitHub では Pull Request をマージする際に 3 種類の方法を選べます。
- Create a merge commit
- Squash and merge
- Rebase and merge
このガイドラインは 1 番を使いましょう、というものです。 Squash というのは Pull Request に含まれる commit 群をひとつの commit にまとめて merge することです。これはあとから見たときに細かい変更意図が読めなくなるので避けましょう。また、 Reabase をすると conflict が発生する可能性があるため、必要であれば手元でマージ先を merge し、 push し直すという手順を取りましょう。
PRs should be merged by the owner, because the PR owner knows the risk better than other people
原則として、 Pull Request のマージはその作成者がやりましょう、という一文です。
Pull Request の影響範囲は作成者が一番よくわかっているため、レビュー承認後に本人の手でマージすると影響が少ないよね、ということです。また、自分で作った Pull Request をマージするのは気分がいいものです。
まとめ
これらのガイドラインは割と長く運用し、携わる開発者の方に都度お願いしてきた内容です。みなさん割と納得して実践してくださるものになっています。
あなたのガイドラインがあればぜひおしえてください。よそのお宅のやりかたを知りたいです。
